feat(ri-exchange): pass offered instance types to reshape analysis - #1246

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

_{Comment @coderabbitai help to get the list of available commands.}

Note: the save-time RIExchangeMode check this PR's body refers to (#1196) is now open as #1243. Merging #1243 first (or together) means a bad stored mode is rejected at save time before this bump makes the scheduled run refuse it.

Independent review 2 (claude-fleet, cold): APPROVE 5bcb13b

Merge order note: this PR bumps the pkg pin from 541e05e to e237996, and that range includes go#578.

• go#578's validateMode (pkg@e237996 exchange/auto.go:189-197, called at :232) makes RunAutoExchange fail closed on any mode other than "manual" or "auto".
• fix(config): validate ri_exchange_mode on PUT /api/config #1243 is the PR meant to guarantee that the platform never stores anything else, and it is still REQUEST_CHANGES: PUT /api/config can still store a case-folded or duplicate-key mode.
• So if this lands first, a row holding such a value with exchange enabled would make the scheduled reshape fail after deploy.
• The ladder is unaffected, because its Mode is hard-coded to "auto" (providers/aws@aeb8a6f ladder/reshape.go:133).
• Land fix(config): validate ri_exchange_mode on PUT /api/config #1243's fix first, or check for such rows before deploying this.

Findings (none blocking on their own):

• [non-blocking] internal/api/handler.go:580-590 - The armed() comment says pkg/exchange sends every mode other than "manual" to auto-execute, including "", "Auto" and typos. At the bumped pin, validateMode fails closed instead. The predicate is still safe, because it is conservative, but the comment is stale.
• [non-blocking] go.mod:87 - The pin range also carries more than the body discloses (it names only fix(api): answer 4xx for bad approval tokens and a missing revoke execution #584 and fix(api): GET /api/purchases/cancel/{id} cancels an execution on a side-effecting GET #578):
  • fix(api): stop approving a purchase on GET #577 (the reservation-name suffix);
  • fix(email): RI-exchange approve/reject email links hit POST-only API routes and return 401 #579, chore(ci): type-check integration-tagged tests in make test #580 and fix(auth): write only the admin-editable columns from UpdateUser #581;
  • the e858df6/0dc660d recfilter unit-counting changes;
  • 0dd6bde (cap reshape targets at owned units).
  • The full api and server suites pass, but the body should list these.
• [non-blocking, by design] internal/api/reshape_offered_types.go:18-28 - If the catalog call fails, the code logs a warning and falls back to the legacy ladder.
  • The issue asks for exactly this ("log a warning with the region and proceed without the catalog. Do not cache that fallback").
  • It matches the pkg contract that empty or nil means legacy (pkg exchange/reshape.go:405-418, auto.go:132-134).
  • On the money path, FindConvertibleOffering and the AWS quote remain the backstop. With IAM missing, the degradation is permanent and visible only as a warning.
• [non-blocking, test gap] Probe M9, the ladder AccountID: clients.accountID replaced with "", survives the new tests. The constructor test does assert c.accountID.
• [note] The catalog is region-level (LocationTypeRegion). A zonal RI's target can be offered in the region but not in its AZ, so that case is still left to the offering lookup or the quote. This is unchanged from before.

Attacked:

• Source, read from the module cache, with the h1 sum matching go.sum: GetValidResourceTypes in providers/aws@aeb8a6f services/ec2/client.go:692-725.
  • It loops over NextToken with MaxResults 1000, and it is scoped to the client's configured region.
  • Any page error returns nil and the error, never a partial list.
  • The helper also discards a partial result returned with an error (M4).
• Analysis, in pkg@e237996 exchange/reshape.go:410-457 and 564-577:
  • Targets come only from the per-family offered ladder.
  • A family missing from the catalog gets a Reason and no target, and RunAutoExchange skips it before any offering lookup (auto.go:286-296).
  • Type names are split with SplitN on "." and not otherwise normalised. AWS enums and RI types are both lowercase.
• Callers: the only reshape and auto-exchange entry points are the API reshape page (handler_ri_exchange.go:1760), the standalone scheduled run (server/handler_ri_exchange.go:178) and the ladder adapter (ladder_write.go:79).
  • ReshapeBuffer (providers/aws ladder/reshape.go:97) reaches the ladder adapter.
  • Region and account are consistent: the API uses the same cfg and ec2Client as the RI listing, and the ladder uses a.region for config, utilisation and the catalog.
• Mutation probes on a scratch copy, each restored and confirmed identical with cmp:
  • M1, the API catalog set to nil: caught.
  • M2, the standalone run drops the catalog: 4 subtests FAIL.
  • M3, the ladder drops the catalog: 4 subtests FAIL.
  • M4, the helper returns a partial result on error: caught.
  • M5, the constructor stubs out the catalog: caught.
  • M6 and M7, the API and ladder warning region blanked: caught.
  • M8, the empty-catalog warning removed: caught.
  • M9, the ladder AccountID blanked: survives. This is the test gap above.
• Fail-before: with the head's go.mod and tests on base, both test packages fail to compile. M1-M3 are the semantic fail-before for each path.

Verified locally, with GOTOOLCHAIN=go1.26.10 and no cloud credentials, under the heavy lock:

• go build ./... and go vet ./internal/api/ ./internal/server/, plain and with -tags integration: ok. gofmt -l on the 12 changed files: clean.
• go test -count=1 ./internal/api/ ./internal/server/: ok. -race on the new tests: ok.
• git merge-tree --write-tree origin/main 5bcb13b against origin/main 6af022f: clean.
• No migration, frontend or terraform changes.

Reviewed by an independent agent with no author context (second reviewer). Review only: nothing was pushed, and the GitHub review state is unchanged.

Merge-order note: this pin bump (541e05e..e237996) includes LeanerCloud/cloud-commitments-go#578, which makes the RI-exchange mode check fail closed. It should merge only after #1243 (ri_exchange_mode validation on PUT /api/config) is fixed and merged, and after the legacy-row query in #1243's body shows no stored mode other than auto/manual. Otherwise a stored Auto or automatic mode with exchange enabled would stop the scheduled reshape. The range also brings in go #577, #579-#581, the recfilter unit-counting change and 0dd6bde (cap reshape targets at owned units); worth listing in the body.

VERDICT: APPROVE 5bcb13b

PR #1246 (draft, @IAmJSD), head 5bcb13b, merge base 503eb2e (3 behind origin/main 2ded648).
Merge-tree vs origin/main: clean (tree 8ce8bdd3). Also clean vs #1243 (9616ae7) and vs #1240. Merged tree builds; vet (plain + integration) clean; go mod tidy -diff clean; go mod verify OK.
Merge gate: must land after #1243 (still an open draft). go#578 makes RunAutoExchange fail closed on any mode other than auto/manual.

Findings (no blockers)

• low (follow-up) frontend/src/riexchange.ts:985,990 - no-target recs (unoffered family, and metal with no factor, now emitted by go#584/0dd6bde) render as "0x " with an active Exchange button. Idle RIs already had this. The PR flags it for the UI owner, but no issue is filed. File one.
• nit internal/api/handler.go:586-600 - the armed() comment says pkg routes every non-"manual" mode to auto; after this bump it fails closed. The predicate is still safe (stricter than needed). fix(config): validate ri_exchange_mode on PUT /api/config #1243 rewrites this block; whichever lands second should fix the wording.
• nit internal/server/handler_ri_exchange.go:19 / ladder_write.go:14 - server imports the exported api.OfferedInstanceTypes. server already imports api (app.go), so there is no new edge. Fine as is.
• Largest commit: e7754dd at 392 changed lines, under the 400 cap.

#1206 checklist vs code

• Pin bump: go.mod pkg 541e05e5fd29 to e237996e45b4. providers stay at aeb8a6f/3def3f8.
• GetValidResourceTypes reaches all three call sites: api/handler_ri_exchange.go:1760 (reshapeEC2Client:50), server/handler_ri_exchange.go:178 (standalone), ladder_write.go:79 (ladder). Region is consistent: the ladder's awsCfg uses WithRegion(a.region); the page uses p.region.
• Fallback: reshape_offered_types.go:18-28 warns with the region on an error or an empty catalog, discards a partial result, and caches nothing.
• Tests cover catalog present, missing family (Reason), error/empty with no caching, for the page, standalone and ladder.

Go-lib range (pkg-only source diff from the module cache)

• fix(api): GET /api/purchases/cancel/{id} cancels an execution on a side-effecting GET #578 validateMode / cancelStalePending: the ladder passes Mode=auto (providers/aws ladder/reshape.go:133). The standalone mode comes from the DB, which defaults to 'manual' and coerces "" to manual (store_postgres.go:313). A cancel failure now aborts the run instead of warning. This is fail-loud and acceptable.
• fix(email): RI-exchange approve/reject email links hit POST-only API routes and return 401 #579 DecodeServiceDetailsFor rejects a payload for an untyped slug. Platform callers: purchase/execution.go:1162, purchase_pricing.go:200-232, handler_purchases.go:3415 (EC2 only), stampComputeCapacity (error-tolerant). The untyped slugs are storage/commitments/other. No pinned provider stamps Details for them (GCP cloudstorage sets none). MemoryDB and nosql now decode to typed structs, so they get engine fallback only. No break.
• fix(api): stop approving a purchase on GET #577 reservation names keep the random suffix (idempotency improves). fix(auth): write only the admin-editable columns from UpdateUser #581 logging SetOutput is race-free. chore(ci): type-check integration-tagged tests in make test #580 retry clamps jitter. The scorer/recfilter/reporter/DropSummary changes are not imported by the platform except through providers.
• 0dd6bde/fix(api): answer 4xx for bad approval tokens and a missing revoke execution #584 cap targets at owned units, skip equal-capacity reshapes, metal returns a Reason. These are covered by updated platform tests.

Attacked / probes (all killed)

• Remove WithOfferedInstanceTypes from the page: CatalogLimitsTargetToOfferedSizes and CatalogMissingFamilySurfacesReason fail.
• Drop OfferedInstanceTypes from the ladder params: TestRIExchangeReshape_OfferedCatalog fails.
• Drop it from the standalone params: the same test fails.
• Return partial types on a fetch error: TestOfferedInstanceTypes fails.
  Results on the merged tree: unit tests for api, server, purchase, scheduler, mocks, database pass (6004). Integration -run "Reshape|RIExchange" for api and server: PASS.
  CI on head: all green (Snyk skipped). It ran on the old base 503eb2e; the merged tree passes locally.

Independent review (IAmJSD's agent, fresh context, given only the diff, the issue and the PR text): VERDICT: REQUEST_CHANGES 5bcb13b14978177846f242c7906241790642b59c

Blocking:

• internal/server/ri_exchange_offered_types_test.go:231: the table test builds its clients with account ID "acct-1". Main gained a 12-digit AWS account-ID check in the exchange store adapter today (fix(db): let ri_exchange_history.account_id hold Azure and GCP identifiers #1170, f56cecb; main internal/server/handler_ri_exchange.go: "refusing to record RI exchange: account ID %q is not a 12-digit AWS account ID"). The PR's Unit Tests ran at 12:29Z, before that landed. After merging current origin/main into this head locally (merges cleanly), TestRIExchangeReshape_OfferedCatalog/standalone/{catalog_limits_the_target_to_offered_sizes, catalog_error_falls_back_to_the_legacy_ladder, empty_catalog_falls_back_to_the_legacy_ladder} fail with "[]" should have 1 item(s). The standalone run is not a dry run, so its pending-record save is refused; the ladder subtests are dry runs and pass. Merging as-is turns main's Unit Tests red. Fix: use a 12-digit ID such as "123456789012" at :121/:142/:231, merge origin/main, and re-run CI.

Non-blocking:

• The pin bump 541e05e5 -> e237996e brings more than go#584 and go#578, and the body doesn't list the rest:

  • common.DecodeServiceDetailsFor now errors on a non-empty payload for an untyped or unknown service slug. It used to return (nil, nil). This reaches the purchase path (internal/purchase/execution.go decode) and recDetailsAvailabilityZone.
  • MemoryDB and NoSQL rows now decode into typed details.
  • RunAutoExchange now fails the run when CancelPendingExchangesByOrigin errors. It used to log a warning and continue.
  • Reservation names keep their random suffix (it is used as the purchase idempotency name).
  • Assorted fixes in recfilter dedupe, scorer validation and retry.

  I checked the providers pinned on main. Every service slug they stamp is typed in the new codec, and GCP storage recs carry no Details, so I found no live break. The full go test -short ./internal/... ./cmd/... passes. Please list these in the body so the deploy note is complete.

• The go#578 mode fail-closed: as the body says, fix(config): validate RIExchangeMode on PUT /api/config #1196 should land first or together with this PR, or stored invalid modes should be checked before deploy.

• Conflicts textually with fix(ri-exchange): run unattended exchanges as manual while 4-eyes is on #1108 in internal/server/ladder_write.go. Both PRs split the ladder adapter (run vs runWithClients), so whichever merges second has to reconcile by hand. The 4-eyes mode downgrade from fix(ri-exchange): run unattended exchanges as manual while 4-eyes is on #1108 must survive in the combined function.

• internal/server now calls api.OfferedInstanceTypes. That works (server already imports api in app.go/http.go), but the helper isn't HTTP-specific. If more server code needs it, internal/exchange or similar would be a better home. Cosmetic.

Attacked:

• Diffed the two pinned pkg module trees file by file (exchange, common, recfilter, scorer, retry, logging, config) to see what the bump brings. Grepped platform callers of the changed APIs and the service slugs the pinned aws/azure/gcp providers emit.
• Fail-before:
  • Dropping exchange.WithOfferedInstanceTypes(offered) fails TestGetReshapeRecommendations_CatalogLimitsTargetToOfferedSizes and TestGetReshapeRecommendations_CatalogMissingFamilySurfacesReason.
  • Dropping the OfferedInstanceTypes param (keeping the fetch) in both server paths fails the 4 catalog-dependent TestRIExchangeReshape_OfferedCatalog subtests.
  • Restored afterwards.
• Ran on the head: go build ./...; go vet with and without -tags integration on api/server; go test -race -short -count=1 ./internal/api/ ./internal/server/ ./internal/mocks/ (ok); go test -short -count=1 ./internal/... ./cmd/... (ok).
• Ran on the head merged with origin/main: build ok, api ok, server FAIL as above; golangci-lint with and without --build-tags integration 0 issues; gocyclo silent.

Body-stale:

• "smallest bump that includes go#584" plus the behavior-change section: the bump also changes service-details decoding, stale-pending cancellation and reservation naming (see above), and the body doesn't mention them.

_{Reviewer: rA1. Bar while the maintainers are away (set by IAmJSD as acting owner): one approving independent review on the head, two for money, auth and security paths.}

Updated the branch. New head: 46cce856 (46cce856cceaebabd066949ba9c46fb5c6fe4e4f).

• Merged origin/main twice with merge commits, first at 7cde88e and then at 26004a6 (fix(purchase): guard the remaining post-CAS full-row saves on status #1111, test(purchase): expect #1245's failure alert in the post-CAS safeFail race test (fixes red main) #1418). There were no conflicts.
• Fixed the fixture that broke under main's 12-digit account ID validation (fix(db): let ri_exchange_history.account_id hold Azure and GCP identifiers #1170). "acct-1" in internal/server/ri_exchange_offered_types_test.go made three TestRIExchangeReshape_OfferedCatalog/standalone/* subtests fail: the exchange record was refused before any assertion ran. It now uses the 123456789012 placeholder. Every assertion is kept, including assert.Equal(t, "123456789012", c.accountID).
• ladder_write.go was not touched.
• Updated the PR body:
  • It now lists three more behavior changes from the pkg pin bump: go#579 service-details decoding, go#578 stale-pending cancellation failing the run, and go#577 reservation naming.
  • It now says this PR must merge after fix(config): validate ri_exchange_mode on PUT /api/config #1243 (ri_exchange_mode validation).
  • It now notes the ladder_write.go conflict with fix(ri-exchange): run unattended exchanges as manual while 4-eyes is on #1108, and that whichever merges second must keep fix(ri-exchange): run unattended exchanges as manual while 4-eyes is on #1108's 4-eyes downgrade.

Checks on the merged tree:

• go build, go vet (plain, integration, e2e): clean.
• golangci-lint v2.10.1 on internal/server and internal/api, with and without --build-tags integration: 0 issues. gocyclo: silent. go mod tidy -diff: clean.
• go test -race -short on ./internal/server/ and ./internal/api/: ok.
• go test -race -tags integration on ./internal/server/ and ./internal/api/: ok on the first merge (7cde88e). The machine was heavily loaded, and TestIntegration_HistorySummary_TotalsCoverAllInScopeRows hit its 3-minute budget while the shared Postgres template was being built. It passed when rerun alone. I did not rerun integration after the second merge, which only brought in purchase/api changes from fix(purchase): guard the remaining post-CAS full-row saves on status #1111 and test(purchase): expect #1245's failure alert in the post-CAS safeFail race test (fixes red main) #1418.
• Secrets scan: clean, and the planted key was flagged. scripts/test-git-secrets-allowlist.sh: 137 PASS.

Independent review (IAmJSD's agent, fresh context, given only the diff, the issue and the PR text): VERDICT: APPROVE 46cce856cceaebabd066949ba9c46fb5c6fe4e4f

Blocking:

• none in the change itself. Merge-order and merge-state items below must be handled before this lands.

Non-blocking:

• Merge state (security-relevant): this head CONFLICTS with main in internal/server/handler_ri_exchange.go and internal/server/ladder_write.go, and in internal/api/handler_purchases.go/its test. On main, fix(ri-exchange): run unattended exchanges as manual while 4-eyes is on #1108 (bdfe866) wraps the standalone mode as fourEyesExchangeMode(cfg, cfg.RIExchangeMode), and fix(server): stop reporting fabricated success on three operator paths #1264 (f05b0e4) made resolveAccountID return an error and abort, replacing the old "unknown". This PR rewrites both call sites through newRIExchangeClients(..., resolveAccountID(ctx, awsCfg)). A careless resolution could drop the 4-eyes downgrade of unattended exchanges, or reintroduce the old resolveAccountID signature. The merge commit needs a fresh review that checks both are kept, in the standalone path and in the ladder adapter.
• Merge order: the body says this must merge after fix(config): validate ri_exchange_mode on PUT /api/config #1243, which is still OPEN. The pinned go#578 makes RunAutoExchange fail closed on any mode other than exactly auto/manual. If this lands first, a stored case-variant mode makes the scheduled reshape fail. That fails loud, not open, so I'm not blocking on it, but land fix(config): validate ri_exchange_mode on PUT /api/config #1243 first or check ri_exchange_mode values before deploying.
• go.mod: this PR bumps pkg to e237996, and fix(onboarding): flag aws-cn accounts and refuse their Savings Plans purchases #1248 bumps providers/aws (which moves pkg to ff6225d). The two will conflict on go.mod/go.sum. With MVS, e237996 has the higher pseudo-version, so whichever lands second has to re-tidy and re-test.
• go#579 (DecodeServiceDetailsFor now rejects a Details payload on an untyped slug): I checked the pinned providers. AWS, Azure and GCP only stamp Details on typed slugs (compute, relational-db, cache, memorydb, nosql, search, data-warehouse and the Savings Plans slugs). GCP storage sets none. I see no break today, but it is a fail-closed change on the purchase path to keep in mind.
• internal/api/handler.go armed() comment: it still says pkg auto-executes any mode other than "manual". After this bump it fails closed. The predicate is still safe; the comment is stale.
• Frontend: recommendations with no target (family not offered in the region) will render as an empty target. The body flags this for the UI owner, but no issue has been filed yet.

Attacked:

• The catalog fetch reaches all three entry points: the API reshape page, the standalone scheduled run and the ladder adapter. Region is consistent (p.region, awsCfg.Region, a.region). On an error or an empty result it falls back to the legacy ladder with a warning, discards a partial result and caches nothing. newRIExchangeClients keeps the exchange client, offering lookup and utilization region wiring the same as before.
• Fail-before: I removed exchange.WithOfferedInstanceTypes(offered) from getReshapeRecommendations and the OfferedInstanceTypes: params from the standalone and ladder runs. TestGetReshapeRecommendations_CatalogLimitsTargetToOfferedSizes, ..._CatalogMissingFamilySurfacesReason and TestRIExchangeReshape_OfferedCatalog (standalone and ladder subtests) failed. They passed after restoring.
• Ran go build ./..., go vet ./..., go vet -tags integration ./..., go test -short -count=1 ./... (whole module, because of the pin bump: all ok), go test -race -short -count=1 on internal/{api,server} (ok), golangci-lint with and without the integration tag (0 issues), gocyclo -over 10 (silent), and go test -race -tags integration -count=1 on internal/{server,api} (ok).

Body-stale:

• "It also conflicts with fix(ri-exchange): run unattended exchanges as manual while 4-eyes is on #1108 in internal/server/ladder_write.go": fix(ri-exchange): run unattended exchanges as manual while 4-eyes is on #1108 has merged (2026-10-25), so this is now a conflict with main, together with fix(server): stop reporting fabricated success on three operator paths #1264 and refactor(ri-exchange): share the convertible RI monthly cost formula #1260/fix(exchange): show unknown-outcome auto-exchanges as needing reconciliation in the notification email #1278 in internal/server/handler_ri_exchange.go.

From earlier reviews (confirmed): #1243 is still in REQUEST_CHANGES, so "merge after #1243" is not yet satisfiable. The stale armed() comment was also raised there.

_{Reviewer: rB24. Bar while the maintainers are away (set by IAmJSD as acting owner): one approving independent review on the head, two for money, auth and security paths.}

Independent review (IAmJSD's agent, fresh context, given only the diff, the issue and the PR text): VERDICT: APPROVE 49a1db522fa487f840850a2b5b44d97cbc75fa25

Blocking:

• none

Non-blocking:

• Every reshape page load, scheduled run and ladder exchange now makes a paginated DescribeInstanceTypeOfferings call with nothing cached. The issue only forbids caching the fallback. A short TTL cache of successful catalogs per region would be a cheap follow-up if page latency or EC2 throttling becomes visible.
• Frontend: recommendations can now come back with an empty target_instance_type and a Reason of "instance family not offered in region". The PR body already hands this to the UI owner. Until the UI change lands, the reshape table may show an empty target cell for these rows.
• The head moved to 49a1db5 since the listed 312072f. The added commit is the x/net v0.62.0 bump, byte-identical to fix(deps): bump golang.org/x/net to v0.62.0 (GO-2026-6860, GO-2026-6866) #1564 (go.mod/go.sum only). I reviewed and tested 49a1db5.

Attacked:

• Money path (scheduled and ladder auto-exchange): the catalog only narrows targets. A family that is not offered is skipped before any offering lookup or quote. A fetch error, an empty catalog, or a partial result returned alongside an error falls back to nil, which is main's legacy-ladder behavior, so a failed catalog call never changes what gets exchanged. GetValidResourceTypes at the pinned go 88b95b6 uses LocationTypeRegion on the region-scoped client and errors out at its page cap instead of returning a truncated list. The 4-eyes downgrade (fourEyesExchangeMode in the standalone run and run() in the ladder adapter) and the fail-closed account resolution are unchanged. ec2:DescribeInstanceTypeOfferings is already granted in the Lambda and Fargate roles, both CloudFormation stacks and the federation templates.
• Fail-before, by mutation, restored after each: (1) dropping the catalog from the standalone RunAutoExchangeParams fails all 4 TestRIExchangeReshape_OfferedCatalog/standalone/* subtests; (2) dropping it from the ladder run() params fails all 4 .../ladder/* subtests; (3) passing WithOfferedInstanceTypes(nil) on the reshape page fails TestGetReshapeRecommendations_CatalogLimitsTargetToOfferedSizes (got m5.2xlarge x1) and ..._CatalogMissingFamilySurfacesReason.
• Ran on 49a1db5: go build ./.... go vet on api and server, with and without -tags integration. go test -race -short -count=1 ./internal/api/ ./internal/server/: ok. golangci-lint v2.10.1 on both packages, with and without the integration tag: 0 issues. gocyclo -over 10 on the changed non-test files: silent. go test -race -tags integration -count=1 ./internal/api/: ok (728s). ./internal/server/ under VM load (load average about 20) had one failure, TestEnsureDB_IsLambdaSingleSourceOfTruth (clone template ... context deadline exceeded, a testcontainers timeout in code this PR does not touch). That test passes when rerun alone, and the -run 'Exchange|Reshape|OfferedCatalog|FourEyes|RIUtilization|Ladder' server integration subset passes.

Body-stale:

• none. go.mod now differs from main by the x/net bump (see above), so "This PR no longer changes the pin" is still true for cloud-commitments-go.

_{Reviewer: rG3. Bar while the maintainers are away (set by IAmJSD as acting owner): one approving independent review on the head, two for money, auth and security paths.}

Independent review (IAmJSD's agent, fresh context, given only the diff, the issue and the PR text): VERDICT: APPROVE 49a1db522fa487f840850a2b5b44d97cbc75fa25

Blocking:

• none

Non-blocking:

• internal/api/reshape_offered_types.go:18: OfferedInstanceTypes calls fetch(ctx) without a nil check. Every production riExchangeClients is built by newRIExchangeClients, so the field is always set today. But a future literal that leaves out offeredInstanceTypes (all the test fixtures had to add it) would panic in the scheduled or ladder exchange run instead of falling back. A fetch == nil case that takes the existing warn-and-return-nil path would cost one line.

• The reshape page now makes a paginated DescribeInstanceTypeOfferings call on every load, uncached. That is the trade-off the PR body states and the issue's no-cached-fallback rule allows. A cache that stores only successful catalogs could be a follow-up if the latency shows.

• (Also raised in an earlier review, and I agree.) Until the UI owner's follow-up lands, recommendations with no target and an "instance family not offered in region" reason will show an empty target cell on the reshape page. No frontend issue has been filed for it yet.

Attacked:

• Checked the pinned go module's ec2svc.Client.GetValidResourceTypes. It queries the full region (LocationTypeRegion), dedupes, and returns an error, never a truncated list, when it hits the pagination cap. A partial catalog therefore can't mark an offered family as "not offered", and an error falls back to the legacy ladder. The API path uses the same p.region the EC2 client was built for, including the defaulted-region case. The server paths keep main's fail-closed account resolution and fix(ri-exchange): run unattended exchanges as manual while 4-eyes is on #1108's 4-eyes run(), with the catalog added on top. ec2:DescribeInstanceTypeOfferings is already granted in the Lambda/Fargate roles, both CloudFormation stacks and the federation templates.
• Ran go build ./..., go vet (with and without -tags integration), go test -race -short -count=1 ./internal/api/ ./internal/server/ (ok) and go test -race -tags integration -run 'Exchange|Reshape|OfferedCatalog|FourEyes|RIUtilization' ./internal/server/ ./internal/api/ (ok). golangci-lint v2.10.1 found 0 issues with and without the integration tag, and gocyclo -over 10 is silent.
• Fail-before: dropping the OfferedInstanceTypes: param from both server call sites fails all 8 TestRIExchangeReshape_OfferedCatalog subtests (standalone and ladder). Passing a nil catalog in getReshapeRecommendations fails TestGetReshapeRecommendations_CatalogLimitsTargetToOfferedSizes and ..._CatalogMissingFamilySurfacesReason. Restored afterwards.

Body-stale:

• none

_{Reviewer: rH1. Bar while the maintainers are away (set by IAmJSD as acting owner): one approving independent review on the head, two for money, auth and security paths.}

Local verification at 49a1db522fa487f840850a2b5b44d97cbc75fa25 merged onto main (bbf0a839), together with #1552, #1553, #1551, #1246, #1548, #1549, #1554, #1560 in that order (combined tree 7f9817d4, Linux, Go 1.26.10): go build ./..., go vet with and without -tags integration, the full go test -race -short -count=1 ./..., and go test -race -tags integration -count=1 on every changed package, against Docker Postgres. All pass. CI Success and pre-commit are green on this head, with 2 approving independent review(s) on it. Nothing was run against any cloud.

Merged by IAmJSD's agent at IAmJSD's request (IAmJSD is acting owner while the maintainers are away), under the bar IAmJSD set: 2 approving independent review(s) on 49a1db52 (two required: money, auth or security path), green CI, and the combined-tree verification above. Squash-merged at exactly 49a1db522fa487f840850a2b5b44d97cbc75fa25.